Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround bindings to textarea.placeholder in IE #5577

Merged
merged 2 commits into from
Jul 31, 2019

Conversation

dfreedm
Copy link
Member

@dfreedm dfreedm commented Jul 30, 2019

Textareas have a very weird bug in IE, where the text of the placeholder
is copied to the content of the textarea when set.

This a creates two bugs:

  1. An unintended binding is made to the textContent of the textarea's
    text child node, meaning updates to the placeholder will be an
    unnecessary binding process in the best case, or an exception thrown
    when updating the text child node in the worst case.
  2. When legacyOptimizations is enabled, the child node of the text
    area is removed when the binding for placeholder is processed and
    removed, leaving a binding to a null node, and throwing exceptions.

Therefore, when we detect this placeholder behavior, we will remove the
textnode before template processing, preventing both bugs.

Textareas have a very weird bug in IE, where the text of the placeholder
is copied to the content of the textarea when set.

This a creates two bugs:
1. An unintended binding is made to the textContent of the textarea's
   text child node, meaning updates to the `placeholder` will be an
   unnecessary binding process in the best case, or an exception thrown
   when updating the text child node in the worst case.
2. When `legacyOptimizations` is enabled, the child node of the text
   area is removed when the binding for `placeholder` is processed and
   removed, leaving a binding to a `null` node, and throwing exceptions.

Therefore, when we detect this placeholder behavior, we will remove the
textnode before template processing, preventing both bugs.
@sorvell
Copy link
Contributor

sorvell commented Jul 30, 2019

For the second problem, this is caused because when textarea has a placeholder in IE, it generates textContent of the same value. Polymer sees this and creates a binding to the textarea.textContent. Polymer removes the placeholder attribute and on IE this removes the textContent of the textarea. Therefore, when the template is stamped, there is no node in the textarea and Polymer cannot find the node to which the textarea.textContent binding should point.

* If the placeholder is a binding, this can break template stamping in two
* ways.
*
* One issue is that when the `placeholder` binding is removed, the textnode
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

placeholder attribute is removed when the binding is processed

Copy link
Contributor

@sorvell sorvell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some small tweaks.

* child of the textarea is deleted, and the template info tries to bind into
* that node.
*
* When `legacyOptimizations` is enabled, the node is removed from the textarea
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the template is stamped and the textarea.textContent binding is processed, no corresponding node is found because it was removed during parsing. An exception is generated when this binding is updated.

* when the `placeholder` binding is processed, leaving an "undefined" cell in
* the binding metadata object.
*
* When `legacyOptimizations` is disabled, the template is cloned before
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When legacyOptimizations is not used, the template is cloned before processing and this changes the above behavior. The cloned template also has a value property set to the placeholder and textContent. This prevents the removal of the textContent when the placeholder attribute is removed. Therefore the exception does not occur. There's an extra unnecessary binding.


function hasPlaceholderBug() {
if (!placeholderBugDetect) {
const t = document.createElement('textarea');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

placeholderBugDetect = true

* @param {!Node} node Check node for placeholder bug
* @return {boolean} True if placeholder is bugged
*/
function shouldFixPlaceholder(node) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest refactoring to fixPlaceholder and apply the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants